Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add orphan inline images below the message body #9326

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add orphan inline images below the message body #9326

wants to merge 2 commits into from

Conversation

bohwaz
Copy link

@bohwaz bohwaz commented Jan 28, 2024

  • In plaintext mode: all inline images will be displayed below the message body
  • In HTML mode: all inline images that are not referenced in the message HTML body will be listed below the message body

This doesn't add more CPU usage as it is using the already called HTML filtering function to discover if an inline part is referenced or not.

This fixes issue #5051 and is a bit better than #9150

@bohwaz
Add orphan inline images below the message body
c56b669 
* In plaintext mode: all inline images will be displayed below the message body
* In HTML mode: all inline images that are **not** referenced in the message HTML body will be listed below the message body

This doesn't add more CPU usage as it is using the already called HTML filtering function to discover if an inline part is referenced or not.

This fixes issue roundcube#5051 and is a bit better than roundcube#9150
@bohwaz bohwaz mentioned this pull request Jan 28, 2024
Open
@bohwaz
Copy link
Author

bohwaz commented Jan 28, 2024

Not sure why CI is failing, but it's not on any code I changed in this PR.

@alecpl
Copy link
Member

alecpl commented May 17, 2024

I think this is wrong because you display the images even if inline_images is false. With this option disabled they should appear on the attachments list, not below the body.

@bohwaz
Copy link
Author

bohwaz commented May 29, 2024

I think this is wrong because you display the images even if inline_images is false. With this option disabled they should appear on the attachments list, not below the body.

@alecpl As far as I understand the code of Roundcube, it is not possible to do what you want currently as the message body is parsed after the display of the attachments, so there is no way to know if the message parts are not in the message body.

This is why I'm appending the parts below the message and not in the message list. Please suggest a code fix if I'm wrong as I'm not familiar with the internals of Roundcube.

@pabzm pabzm mentioned this pull request May 30, 2024
Merged
@pabzm
Copy link
Member

pabzm commented May 30, 2024

@bohwaz The references are actually collected when parsing the message. You can collect them from there, as #9472 does. If that PR gets merged (and doesn't "accidentally" fix the issues you mentioned here), you'll be able to easily implement your idea, I think.

@bohwaz
Copy link
Author

bohwaz commented Jun 5, 2024

I think your PR solves this issue as well, but I have zero time to invest in this currently. I'll check this later maybe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bohwaz @alecpl @pabzm